👋 Welcome back mstrauss! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@mstr2 This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8343398: Add reducedData preference

Reviewed-by: angorya, kcr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 1 new commit pushed to the master branch:

• 98916fe: 8342233: Regression: TextInputControl selection is backwards in RTL mode

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@mstr2
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

@mstr2 has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@mstr2 please create a CSR request for issue JDK-8343398 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

Webrevs

• 11: Full - Incremental (b1635719)
• 10: Full - Incremental (6f015a47)
• 09: Full - Incremental (b7840bd6)
• 08: Full - Incremental (f8384925)
• 07: Full - Incremental (7feba0bf)
• 06: Full - Incremental (fd4846a9)
• 05: Full - Incremental (cbd3cb63)
• 04: Full - Incremental (4bfcb337)
• 03: Full - Incremental (455613f0)
• 02: Full - Incremental (8b8b1bdb)
• 01: Full - Incremental (5c2e255f)
• 00: Full (0245c2e4)

This PR also includes a small refactor of the GTK and macOS PlatformSupport classes.

The previous implementations had some parts of the change notification mechanism implemented outside of PlatformSupport in their respective GlassApplication class. The refactor moves this into PlatformSupport, which centralizes all aspects related to platform preference change detection.

Does not seem to work on macOS 15.1.1 M1:

Does not seem to work on macOS 15.1.1 M1:

It works on my 14.6 machine. Let me try upgrading and testing again...

I might suggest adding the instructions on how to control the platform settings as comments somewhere in the implementation.

Windows: Settings -> Network and Internet -> Ethernet/WiFi -> Metered connection
macOS: Settings -> Network -> Ethernet/WiFi -> Network Settings -> Low data mode
Ubuntu: Settings -> Network -> Wired/WiFi -> Metered connection

(I've merged the latest master prior to testing)

Does not seem to work on macOS 15.1.1 M1:

It also works on 15.1.1 on my machine. Just to check the obvious: did you close the WiFi Details window? The settings are only applied when you click OK.

Update: I was getting interference from the VPN application. Getting off the VPN helped - I see the property and macOS.*.currentPathConstrained key being toggled.

(macOS.*.currentPathExpensive remains false).

Update: I was getting interference from the VPN application. Getting off the VPN helped - I see the property and macOS.*.currentPathConstrained key being toggled.

(macOS.*.currentPathExpensive remains false).

That's the expected result.

I might suggest adding the instructions on how to control the platform settings as comments somewhere in the implementation.

Windows: Settings -> Network and Internet -> Ethernet/WiFi -> Metered connection
macOS: Settings -> Network -> Ethernet/WiFi -> Network Settings -> Low data mode
Ubuntu: Settings -> Network -> Wired/WiFi -> Metered connection

I feel this wouldn't age well, considering that the particular UI used by the different operating systems has often changed over the years.

I feel this wouldn't age well, considering that the particular UI used by the different operating systems has often changed over the years.

I think it has merit. Now that we need to re-test the functionality, it would have helped. Even when something changes, it will be easier to look up a few missing pieces than dig through the PRs...

I feel this wouldn't age well, considering that the particular UI used by the different operating systems has often changed over the years.

I think it has merit. Now that we need to re-test the functionality, it would have helped. Even when something changes, it will be easier to look up a few missing pieces than dig through the PRs...

I think this sort of information is better suited for a Wiki.

Btw, I can review the native code changes. Just not for a couple days.

tested macOS behavior, all is well.

BTW, I've updated the Monkey Tester to highlight changes in Tools -> Platform Preferences Monitor.

https://github.com/andy-goryachev-oracle/MonkeyTest

The headful test run passed on Windows. While doing manual testing of other settings (since the notification logic has changed), I discovered a reproducible crash on Windows 11:

1. Run the manual PlatformPreferencesChangedTest app (or any other JavaFX app)
2. Toggle Settings --> Accessibility --> Visual effects --> Animation effects

JavaFX will crash, likely in response to the notification of a change in animation effect.

The headful test run passed on Windows. While doing manual testing of other settings (since the notification logic has changed), I discovered a reproducible crash on Windows 11:

1. Run the manual PlatformPreferencesChangedTest app (or any other JavaFX app)
2. Toggle Settings --> Accessibility --> Visual effects --> Animation effects

JavaFX will crash, likely in response to the notification of a change in animation effect.

This is a weird one. I've narrowed it down to INetworkInformation::GetInternetConnectionProfile returning null, and then crashing the app. Now, this method is specified to return null if there's no connection profile, so a null-check is obviously in order.

What is strange is that at least on my machine, GetInternetConnectionProfile always returns a non-null value except when the method is called in response to receiving WM_SETCLIENTAREAANIMATION. Note that the method doesn't fail with an error, it simply returns null. I've checked all "obvious" things that might go wrong, such as calling the method on a wrong thread (not the case, it's always the same thread, regardless of whether we react to a window message or a WinRT callback).

The quick fix in this case is simply not calling the API in question in response to the unrelated WM_SETCLIENTAREAANIMATION message. I've added a parameter to collectPreferences() that indicates what type of preferences we're going to collect.

I did notice one thing in macOS while reviewing the changes in PlatformSupport.m. The PlatformSupport object stays around after the GlassApplication is terminated via finishTerminating. It will continue to process platform events, including making Java upcalls even after the FX toolkit is stopped.

Correction: the listeners no longer get notified after the FX toolkit is stopped, perhaps due to the fix forJDK-8335630. Needs more investigation.

Correction: the listeners no longer get notified after the FX toolkit is stopped, perhaps due to the fix forJDK-8335630. Needs more investigation.

This is not related to JDK-8335630. The notification does get delivered, but ends up going through the "else" block in the following code in PlatformImpl::updatePreferences:

        if (isFxApplicationThread()) {
            checkHighContrastThemeChanged(preferences);
            platformPreferences.update(preferences);
        } else {
            // Make a defensive copy in case the caller of this method decides to re-use or
            // modify its preferences map after the method returns. Don't use Map.copyOf
            // because the preferences map may contain null values.
            Map<String, Object> preferencesCopy = new HashMap<>(preferences);
            runLater(() -> updatePreferences(preferencesCopy));
        }

This happens because isFxApplicationThread() returns false after the toolkit is stopped (see Application::finishTerminating). The runLater at the end of the else block never runs and the runnable is discarded, so the else block ends up being a no-op.

So I think my recommendation of "maybe file a P4 bug to look at later" is the most that is needed here.

The changes all look good now.

I did notice one thing in macOS while reviewing the changes in PlatformSupport.m. The PlatformSupport object stays around after the GlassApplication is terminated via finishTerminating. It will continue to process platform events, including making Java upcalls even after the FX toolkit is stopped. You would only see this with a Swing interop app that initializes FX and then later terminates it while keeping AWT alive. I think it is preexisting, and doesn't seem to be causing any problems. It might be worth a P4 follow-up issue to investigate.

I've added some code that ensures that the PlatformSupport instance stops event processing and is released when GlassApplication shuts down. I've verified this by logging whether PlatformSupport::dealloc is called, which it is.

I highly recommend syncing up with the latest master once the PR branch is out for a prolonged period. This will ensure that the new changes do not interfere with the latest updates, and it also saves the reviewer's time as they need to merge in order to do a good review (times N reviewers).

An observation: when running HelloJFXPanel, noticed that the FX animation restarts each time the appearance preference gets changed between light and dark modes. This might be an existing issue since it can also be observed in the master branch.
I am not entirely sure this is expected, since neither the color of the animated rectangle nor the background color changes. It's unrelated to the changes in this PR (but might be related to the way the platform notifications are being processed).

When I run it, it doesn't reset, it seems to jump -- often, but not always, to the beginning or end. In any case, it's completely unrelated to platform notifications: I just checked and it behaves the same way on JDK 8, which has no platform preference support.

Another observation related to the same animation, and also can be observed with the master branch: when one runs the HelloJFXPanel app, the initial animation is smooth and is not affected by mouse over. Once the platform light/dark mode preference gets changed, however, the movement becomes jerky on mouse over.

I can't reproduce this on my system. If you can characterize this, please file a new bug.

More on jerky animation: it's unrelated to the platform preferences change (which is good, for this PR anyway).

It seems the jerky movement appears after HelloJFXPanel loses focus to some other window. Switch back, mouse over and the jerkiness starts. Happens even with the second external monitor disconnected.

/integrate

Going to push as commit 76d5e1a.
Since your change was applied there has been 1 commit pushed to the master branch:

• 98916fe: 8342233: Regression: TextInputControl selection is backwards in RTL mode

Your commit was automatically rebased without conflicts.

@mstr2 Pushed as commit 76d5e1a.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.